Skip to content

Fix internal error when using first class callable with NullsafeMethodCall#5888

Open
phpstan-bot wants to merge 15 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3pnp41f
Open

Fix internal error when using first class callable with NullsafeMethodCall#5888
phpstan-bot wants to merge 15 commits into
phpstan:2.2.xfrom
phpstan-bot:create-pull-request/patch-3pnp41f

Conversation

@phpstan-bot

@phpstan-bot phpstan-bot commented Jun 16, 2026 •
edited by staabm
Loading

Copy link
Copy Markdown
Collaborator

Summary

Using the first-class callable syntax together with the nullsafe operator
($foo?->bar(...)) crashed PHPStan with Internal error.. While this is a
fatal error in PHP itself ("Cannot combine nullsafe operator with Closure
creation"), PHPStan must not crash on it. This change makes the analyser treat
such an expression as a regular method-call first-class callable.

Fixes phpstan/phpstan#9746

@staabm @phpstan-bot
Treat first-class callable NullsafeMethodCall as a MethodCall ins…
da17bb9 
…tead of throwing

- `NodeScopeResolver::processExprNode()` handled the first-class callable
  syntax (`foo(...)`) for `FuncCall`, `MethodCall`, `StaticCall` and `New_`,
  but `NullsafeMethodCall` (`$foo?->bar(...)`) fell through to the `else`
  branch and threw `ShouldNotHappenException`, surfacing as an internal error.
- `NullsafeMethodCall` is the only `Expr\CallLike` subclass that was missing
  from this branch. It is added by wrapping the call in a `MethodCallableNode`
  built from an equivalent `MethodCall`, since `$foo?->bar(...)` is a fatal
  error in PHP ("Cannot combine nullsafe operator with Closure creation") and
  must not crash the analyser.
- Type resolution already routed correctly through `NullsafeMethodCallHandler`,
  so no internal error occurs anywhere the expression is typed (assignment,
  argument, array element, return).
staabm
staabm requested changes Jun 16, 2026
View reviewed changes

@staabm staabm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crash bug fixes need a AnalyserIntegrationTest

@phpstan-bot @claude
Add AnalyserIntegrationTest for first-class callable NullsafeMethodCa…
a2149b4 
…ll crash

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@phpstan-bot

phpstan-bot commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Done. I addressed staabm's review by adding the requested AnalyserIntegrationTest coverage for the crash fix.

What I did

  1. New data file tests/PHPStan/Analyser/data/bug-9746.php — reproduces the original crash path (the first-class callable NullsafeMethodCall used as a statement, which is exactly what hit CallToMethodStatementWithoutSideEffectsRule → MutatingScope::resolveType() → ShouldNotHappenException in the issue's stack trace).

  2. New test testBug9746() in AnalyserIntegrationTest — runs the full analyser over the file and asserts the three resulting errors, confirming analysis completes without an internal error.

Verification

  • With the fix: test passes (10 assertions).
  • Without the fix (temporarily reverted the committed NullsafeMethodCall branch): the test errors with the ShouldNotHappenException internal error — confirming it genuinely catches the crash.
  • make phpstan: No errors.

Committed as a2149b421 and pushed to create-pull-request/patch-3pnp41f. The existing nsrt/bug-9746.php type-inference test remains as-is; this adds the integration-level crash regression the reviewer asked for.

staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread tests/PHPStan/Analyser/data/bug-9746.php
@phpstan-bot @claude
Report "Cannot combine nullsafe operator with Closure creation" for f…
d31bdac 
…irst-class callable NullsafeMethodCall

$foo?->bar(...) is a fatal error in PHP, but PHPStan only silently
treated it as a regular method-call first-class callable. Carry a
nullsafe flag on MethodCallableNode and report the non-ignorable error
from MethodCallableRule.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread src/Node/MethodCallableNode.php Outdated
phpstan-bot and others added 2 commits June 17, 2026 05:31
@phpstan-bot @claude
Carry first-class callable nullsafe info in MethodCallableNode's orig…
acc8721 
…inal node

Instead of a separate `$nullsafe` constructor flag, store the original
`NullsafeMethodCall` in `MethodCallableNode::$originalNode` and derive
`isNullsafe()` from an `instanceof` check. ClassStatementsGatherer converts
the nullsafe node into a regular `MethodCall` when collecting it.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm
assert based on original-node
b3ee78e
staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread src/Analyser/NodeScopeResolver.php Outdated
phpstan-bot and others added 2 commits June 17, 2026 06:21
@phpstan-bot @claude
Introduce NullsafeFirstClassCallableNode for $foo?->bar(...)
d2288b9 
Represent the nullsafe first-class callable in its own virtual node and
report "Cannot combine nullsafe operator with Closure creation." from a
dedicated NullsafeFirstClassCallableRule, instead of overloading
MethodCallableNode/MethodCallableRule with a nullsafe branch.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@phpstan-bot @claude
Test NullsafeFirstClassCallableRule
daa1ab4 
Move the nullsafe first-class callable coverage out of MethodCallableRuleTest
into a dedicated NullsafeFirstClassCallableRuleTest.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@staabm staabm changed the title Treat first-class callable NullsafeMethodCall as a MethodCall instead of throwing Fix internal error when using first class callable with NullsafeMethodCall Jun 17, 2026
@staabm staabm requested a review from VincentLanglet June 17, 2026 08:24
@VincentLanglet

VincentLanglet commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Since there is a new rule, it has to be approved by ondrej

@staabm

staabm commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

@ondrejmirtes this implements a simple new rule. would be great to have your approval.

staabm
staabm reviewed Jun 17, 2026
View reviewed changes
Comment thread tests/PHPStan/Rules/Methods/data/nullsafe-first-class-callable.php
@phpstan-bot @claude
Test dynamic first-class callable nullsafe method call
3bf98fb 
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@VincentLanglet

VincentLanglet commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

@ondrejmirtes this implements a simple new rule. would be great to have your approval.

Maybe you could split the PR in two and merge the bugfix @staabm ?

@staabm

staabm commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

I don't think its worth splitting up, as the internal error exists since 2023 and it seems noone else triggered it yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@staabm staabm staabm approved these changes

@VincentLanglet VincentLanglet VincentLanglet approved these changes

@ondrejmirtes ondrejmirtes Awaiting requested review from ondrejmirtes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Internal error when using first class callable with NullsafeMethodCall

3 participants

@phpstan-bot @VincentLanglet @staabm